Go sdk #579
Go sdk #579
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive Go SDK for A2UI, along with a sample agent implementation. The changes are well-structured and cover core SDK functionality, testing, and a practical usage example. My review focuses on improving code efficiency, robustness, and removing redundant or dead code. Key suggestions include optimizing type assertions, fixing a logging issue, enhancing type handling for tool parameters, and simplifying HTTP header processing.
a2a_agents/go/a2ui/a2ui.go
Outdated
| func GetA2UIDataPart(part a2a.Part) (*a2a.DataPart, error) { | ||
| if IsA2UIPart(part) { | ||
| return part.(*a2a.DataPart), nil | ||
| } | ||
| return nil, fmt.Errorf("part is not an A2UI part") |
There was a problem hiding this comment.
The current implementation of GetA2UIDataPart calls IsA2UIPart, which performs a type assertion and checks metadata. If it returns true, GetA2UIDataPart then performs the same type assertion again. This is inefficient.
You can refactor this function to perform the checks directly, avoiding the redundant type assertion and function call, which improves performance.
func GetA2UIDataPart(part a2a.Part) (*a2a.DataPart, error) {
dp, ok := part.(*a2a.DataPart)
if !ok {
return nil, fmt.Errorf("part is not a DataPart")
}
if dp.Metadata != nil && dp.Metadata[MIMETypeKey] == MIMEType {
return dp, nil
}
return nil, fmt.Errorf("part is not an A2UI part")
}| for _, message := range jsonData { | ||
| log.Printf("Found %d messages. Creating individual DataParts.", len(jsonData)) | ||
| if msgMap, ok := message.(map[string]interface{}); ok { | ||
| finalParts = append(finalParts, CreateA2UIPart(msgMap)) | ||
| } | ||
| } |
There was a problem hiding this comment.
The log statement inside this for loop will be printed for every message in jsonData, each time reporting the total number of messages. This will result in confusing and repetitive log output. For example, if there are 3 messages, it will print "Found 3 messages..." three times. This log statement should be moved outside the loop to be printed only once.
| for _, message := range jsonData { | |
| log.Printf("Found %d messages. Creating individual DataParts.", len(jsonData)) | |
| if msgMap, ok := message.(map[string]interface{}); ok { | |
| finalParts = append(finalParts, CreateA2UIPart(msgMap)) | |
| } | |
| } | |
| log.Printf("Found %d messages. Creating individual DataParts.", len(jsonData)) | |
| for _, message := range jsonData { | |
| if msgMap, ok := message.(map[string]interface{}); ok { | |
| finalParts = append(finalParts, CreateA2UIPart(msgMap)) | |
| } | |
| } |
| func (a *RizzchartsAgent) LoadExample(ctx context.Context, path string, a2uiSchema map[string]interface{}) (map[string]interface{}, error) { | ||
| // Assuming local file system for simplicity in this sample | ||
| data, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to read example file %s: %w", path, err) | ||
| } | ||
|
|
||
| var exampleJSON interface{} // Can be map or slice | ||
| if err := json.Unmarshal(data, &exampleJSON); err != nil { | ||
| return nil, fmt.Errorf("failed to parse example JSON: %w", err) | ||
| } | ||
|
|
||
| // Validate against schema | ||
| schemaBytes, err := json.Marshal(a2uiSchema) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| c := jsonschema.NewCompiler() | ||
| if err := c.AddResource("schema.json", strings.NewReader(string(schemaBytes))); err != nil { | ||
| return nil, err | ||
| } | ||
| schema, err := c.Compile("schema.json") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if err := schema.Validate(exampleJSON); err != nil { | ||
| return nil, fmt.Errorf("example validation failed: %w", err) | ||
| } | ||
|
|
||
| // Return as map if it's an object, or handle slice if needed. | ||
| // The Python code returns dict[str, Any], but the input can be a list. | ||
| // For A2UI examples which are arrays of messages, we usually process them as interface{}. | ||
| // Here we return interface{} to be flexible, but signature says map[string]interface{}. | ||
| // Let's adjust to interface{}. | ||
| return nil, nil // Placeholder, logic below | ||
| } |
There was a problem hiding this comment.
The function LoadExample appears to be dead code. It is defined but never called within the project; the function LoadExampleGeneric is used instead. To improve code clarity and maintainability, this unused function should be removed.
| if txtPart, ok := p.(*a2a.TextPart); ok { | ||
| userText = txtPart.Text | ||
| } else if txtPart, ok := p.(a2a.TextPart); ok { | ||
| userText = txtPart.Text | ||
| } |
There was a problem hiding this comment.
The else if block checking for p.(a2a.TextPart) is unreachable. The reqCtx.Message.Parts slice contains elements of type a2a.Part, which is an interface. Concrete types that implement this interface are stored as pointers (e.g., *a2a.TextPart). The first check p.(*a2a.TextPart) correctly handles the pointer case. A type assertion to a non-pointer struct value a2a.TextPart will not succeed in this context. This redundant check can be removed to simplify the code.
if txtPart, ok := p.(*a2a.TextPart); ok {
userText = txtPart.Text
}| s := &genai.Schema{Type: genai.TypeString} // Default to string | ||
| if typeStr, ok := defMap["type"].(string); ok { | ||
| if typeStr == "string" { | ||
| s.Type = genai.TypeString | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for converting tool parameters to the genai.Schema format is not very robust as it only handles the string type. It defaults to string and only explicitly checks for the string type. This will fail for tools with non-string parameters (e.g., number, boolean). While this works for the current sample, making the type handling more comprehensive would make the executor more robust and reusable for other tools.
s := &genai.Schema{}
if typeStr, ok := defMap["type"].(string); ok {
switch typeStr {
case "number", "integer":
s.Type = genai.TypeNumber
case "boolean":
s.Type = genai.TypeBoolean
default:
s.Type = genai.TypeString // Default to string for unknown types
}
} else {
s.Type = genai.TypeString // Default to string if type is not specified
}| exts := r.Header.Values("X-A2a-Extensions") | ||
| if len(exts) == 0 { | ||
| exts = r.Header.Values("X-A2A-Extensions") | ||
| } |
There was a problem hiding this comment.
The current logic for retrieving the X-A2a-Extensions header is redundant. The http.Header.Values method is case-insensitive by specification, so checking for multiple casings (X-A2a-Extensions and then X-A2A-Extensions) is unnecessary. A single call is sufficient and makes the code cleaner.
| exts := r.Header.Values("X-A2a-Extensions") | |
| if len(exts) == 0 { | |
| exts = r.Header.Values("X-A2A-Extensions") | |
| } | |
| extensions := r.Header.Values("X-A2a-Extensions") |
1 participant
Description
Go SDK
This has been generated by Gemini to match the features and
functionality of the Python SDK.
The Rizzcharts sample has also been regenerated in Go, using the Go SDK
as a demonstration.
Pre-launch Checklist